Use unmodified official ClangFormat configuration as base formatter configuration #1324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
per1234
added
type: enhancement
We could move the default formatter JSON into a dedicated file if it helps the synchronization. Consider the following folder structure: and then from code: function styleJson({
TabWidth,
UseTab,
}: ClangFormatOptions): Record<string, unknown> {
// Source: https://github.com/arduino/tooling-project-assets/tree/main/other/clang-format-configuration
const defaultConfig = require('./default-formatter-config.json'); // 1. require the JSON
return {
...defaultConfig,
TabWidth, // 2. override the default values with the user-defined ones
UseTab,
};
}
I think this will make the config synchronization easier and we do not have to touch code when we update the config. What do you think? Click here, I created and formatted the JSON so that you can copy-paste it into a
|
|
Thanks so much @kittaakos. That approach is ideal for facilitating the maintenance of the formatter configuration. I have now updated the PR according to your suggestions: |
…onfiguration The Arduino IDE's "Auto Format" feature is configured to produce the standard Arduino sketch formatting style by default. The Arduino IDE editor's default settings are compliant with that style. However, the user may adjust the editor settings. In this case, the Arduino IDE automatically adjusts the Auto Format configuration to align with the user's preferences. The formatter configuration is consumed by several other projects in addition to the Arduino IDE. For this reason, the configuration is hosted and maintained in a centralized location, from which it is pulled by all projects that use it. Previously, the adjustment of the Arduino IDE formatter configuration according to the editor settings was integrated into the configuration object itself. This meant that the standardized configuration had to be modified each time it was pulled in to sync from the upstream source. Moving the base formatter configuration object to a dedicated file, separated from the handling and adjustment code allows syncs to be done by simply replacing the existing configuration file with the one automatically generated by the CI system of the repository where the source configuration is hosted.
kittaakos
reviewed
Aug 16, 2022
| @@ -147,189 +147,10 @@ function styleJson({ | |||
| UseTab, | |||
| }: ClangFormatOptions): Record<string, unknown> { | |||
| // Source: https://github.com/arduino/tooling-project-assets/tree/main/other/clang-format-configuration | |||
| const defaultConfig = require('./default-formatter-config.json'); // 1. require the JSON | |||
There was a problem hiding this comment.
This line should be changed to, it's my fault. I found it only when verifying the behavior:
const defaultConfig = require('../../src/node/default-formatter-config.json');Explanation:
We write TS code that will be transpiled to JS code. IDE2 does not run TS code, only JS code. The sources (TS files, JSON, .etc) are in src, the transpiled JS code is in lib.
The TS sources and the JSON config are here:
arduino-ide-extension/src/node/clang-formatter.ts
arduino-ide-extension/src/node/default-formatter-config.json
The TS code will be JS, and the module location will be these:
arduino-ide-extension/lib/node/clang-formatter.d.ts
arduino-ide-extension/lib/node/clang-formatter.d.ts.map
arduino-ide-extension/lib/node/clang-formatter.js
Notice the src and lib differences in the path. JSON files will remain in src at runtime, JS files will be in lib folder. To be able to require the JSON file from the transpiled JS, the require path must be changed from ./default-formatter-config.json to ../../src/node/default-formatter-config.json. If this is not clear, please let me know.
Also, please remove the // 1. require the JSON and the // 2. override the default values with the user-defined ones comments. I added them for explanation purposes.
Thank you!
There was a problem hiding this comment.
This line should be changed
Thanks. I have done it here: becb901
JSON files will remain in src at runtime, JS files will be in lib folder.
I saw that when I went to test the build (I think I somehow ended up still old source code when I ran it from source).
I was going down the path mentioned here:
https://stackoverflow.com/a/53629381/7059512
(adding something like "src/node/*.json" to the include field of tsconfig.json)
please remove the // 1. require the JSON and the // 2. override the default values with the user-defined ones comments.
OK, done here: aff3560
kittaakos
approved these changes
Aug 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Arduino IDE's "Auto Format" feature is configured to produce the standard Arduino sketch formatting style by default.
The Arduino IDE editor's default settings are compliant with that style. However, the user may adjust the editor settings. In this case, the Arduino IDE automatically adjusts the "Auto Format" configuration to align with the user's preferences.
The formatter configuration is consumed by several other projects in addition to the Arduino IDE. For this reason, the configuration is hosted and maintained in a centralized location, from which it is pulled by all projects that use it.
Motivation
The adjustment of the Arduino IDE formatter configuration according to the editor settings is integrated into the configuration object itself:
arduino-ide/arduino-ide-extension/src/node/clang-formatter.ts
Lines 324 to 326 in 75e00c2
This means that the standardized configuration has to be modified each time it is pulled in to sync from the upstream source.
Change description
Move the adjustment code out of the base formatter configuration object to allow syncs to be done via a pure copy/paste replacement.
Other information
Example of a configuration sync: #1285
Reviewer checklist